-
Notifications
You must be signed in to change notification settings - Fork 418
Add a method to avoid re-persisting monitors during startup #3996
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add a method to avoid re-persisting monitors during startup #3996
Conversation
👋 Thanks for assigning @tnull as a reviewer! |
Prior to LDK 0.1, in rare cases we could replay payment claims to `ChannelMonitor`s on startup, which we then expected to be persisted prior to normal node operation. This required re-persisting `ChannelMonitor`s after deserializing the `ChannelManager`, delaying startup in some cases substantially. In 0.1 we fixed this, moving claim replays to the background to run after the `ChannelManager` starts operating (and only updating/persisting changes to the `ChannelMonitor`s which need it). However, we didn't actually enable this meaningfully in our API - nearly all users use our `ChainMonitor` and the only way to get a chanel into `ChainMonitor` is through the normal flow which expects to persist. Here we add a simple method to load `ChannelMonitor`s into the `ChainMonitor` without persisting them.
f968fa1
to
e45bfed
Compare
Yeah, I'd also lean towards not backporting this, also as it changes API (even though just extending). |
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3996 +/- ##
=======================================
Coverage 88.93% 88.94%
=======================================
Files 174 174
Lines 124575 124629 +54
Branches 124575 124629 +54
=======================================
+ Hits 110790 110850 +60
+ Misses 11287 11280 -7
- Partials 2498 2499 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Prior to LDK 0.1, in rare cases we could replay payment claims to
ChannelMonitor
s on startup, which we then expected to be persisted prior to normal node operation. This required re-persistingChannelMonitor
s after deserializing theChannelManager
, delaying startup in some cases substantially.In 0.1 we fixed this, moving claim replays to the background to run after the
ChannelManager
starts operating (and only updating/persisting changes to theChannelMonitor
s which need it). However, we didn't actually enable this meaningfully in our API - nearly all users use ourChainMonitor
and the only way to get a chanel intoChainMonitor
is through the normal flow which expects to persist.Here we add a simple method to load
ChannelMonitor
s into theChainMonitor
without persisting them.I'm kinda on the fence about backporting this. On the one hand its a pretty nontrivial speedup for users who switch to the new interface. On the other hand, it is a "feature" and its kinda weird to backport a feature.